fix(server-nestjs): reuse Vault AppRole secret-id instead of minting on every sync - #2626
fix(server-nestjs): reuse Vault AppRole secret-id instead of minting on every sync#2626shikanime wants to merge 1 commit into
Conversation
297676a to
e61aa06
Compare
shikanime
left a comment
shikanime
left a comment
There was a problem hiding this comment.
Verdict : Changements demandés — conflit de fusion + gardes déjà sur main (PR propriétaire : commentaire, non request-changes).
- mergeable=CONFLICTING — [🔴 Bloquant] La PR cible
mainmais re-implémenteisVaultNotFound/isVaultBadRequest/isNexusNotFound/isGitbeakerUnauthorizedqui y sont déjà (commitec9c18a914). Le conflit porte précisément survault.utils.ts,nexus.utils.ts,gitlab.utils.ts. Rebasez surmain: gardez uniquementgenerateAppRoleSecretIdPath+ la logique get-or-create deensureAuthApproleRoleSecretId. - vault-client.service.ts:401-456 — [✨ Éloge] Le get-or-create du secret-id AppRole (relit le KV
APPROLE_SECRET_ID, réutilise si présent, sinon mint + persiste) résout exactement #2622 : plus de nouveau commitvalues.yamlà chaque sync. Tests first/second-sync bien couverts. - argocd.service.ts:405 + spec — [🟢 Conforme] Le renommage
createAuthApproleRoleSecretId→ensureAuthApproleRoleSecretIdest cohérent partout (service + 5 mocks de spec).
Corrigez le conflit par un rebase sur main avant fusion.
…on every sync Refs #2622 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Ie3d3b7df1e0539c02d6a215ce7eff82a6a6a6964
e61aa06 to
f8f16bd
Compare
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
shikanime
left a comment
shikanime
left a comment
There was a problem hiding this comment.
Verdict : Approuvé — la PR reste en draft dans l'attente de la revue humaine.
Correction de la cause racine bien ciblée : le secret-id AppRole est relu dans le KV du projet avant toute nouvelle frappe, ce qui supprime l'accumulation de secret-id et les commits values.yaml fantômes à chaque cron ou upsert. Les primitives read/write existantes sont réutilisées telles quelles et les deux tests verrouillent précisément la Définition du fini de #2622.
|
|
||
| @StartActiveSpan() | ||
| async createAuthApproleRoleSecretId(roleName: string) { | ||
| async ensureAuthApproleRoleSecretId(roleName: string) { |
There was a problem hiding this comment.
✨ Éloge — get-or-create sur le KV projet : la relecture avant frappe rend la synchronisation idempotente sans nouveau chemin d'API, et le role-id conserve son GET inchangé.
| this.logger.verbose(`Creating Vault AppRole secret-id for ${roleName}`) | ||
| const response = await this.http.fetch<VaultSecretIdResponse>(path, { method: 'POST' }) | ||
| const secretId = response?.data?.secret_id | ||
| if (!secretId) { |
There was a problem hiding this comment.
⚪ Suggestion — si l'écriture KV échoue après la frappe, le secret-id fraîchement frappé reste orphelin dans Vault ; le run suivant relit le KV, ne trouve rien et refrappe, donc l'idempotence se rétablit seule avec un résidu borné. Acceptable en l'état.
1 participant
Issues liées
Quel est le comportement actuel ?
À chaque synchronisation (project.upsert, cron),
generateVaultValuesappellecreateAuthApproleRoleSecretId, qui émet un nouveau secret-id AppRole viaPOST auth/approle/role/{role}/secret-id. Le secret-id n'est ni relu ni réutilisé : il s'accumule dans Vault à chaque exécution, et son renouvellement systématique rend la diff de contenu desvaluestoujours sale, d'où un nouveau commitvalues.yamlà chaque passage même sans changement de configuration.Comportement attendu
Le secret-id est idempotent : une seconde synchronisation sans changement de configuration ne produit ni nouveau secret-id ni nouveau commit
values.yaml. Le role-id continue d'être relu via GET (chemin inchangé).Changements
createAuthApproleRoleSecretIddevient un get-or-create : il relit un secret-id persisté dans le KV Vault du projet ; s'il existe, il le réutilise, sinon il le crée puis le persiste.APPROLE_SECRET_ID(aux côtés des autres identifiants du projet).